Tests and tweaks to the select menu widget #1019
Conversation
in multiple select mode. Also improved / refactored some implementation details.
|
Thanks @madebydna! I'm still not quite free of other deadlines but this sounds great. As for the expected behavior when the prompt is selected, it feels natural to me that the default is all-selected and is easily chosen with the prompt. Even though it may seem inconsistent or illogical, it's the most common choice and IMO is similar to the way the other ordinal selections work (pie, row, and ordinal bar charts). I am considering also adding a "reverse" or "invert" option, since excluding some options is also a common thing to do. (Right now I'm using the widget to select certain node types to filter out in a network diagram in order to make it readable.) |
|
We should also test that the selected options are the same as the filters for the multi case. I think I found a bug where when the filters are set it only adds items and doesn't remove any. |
here is another thing that should be tested (re: #1019)
There are now tests for the no-values, one-value, and multiple-values case in multiple select mode.
Also improved / refactored some implementation details. @gordonwoodhull, as per your suggestions I changed the implementation of
_chart.onChangeto always callreplaceFilterwith an array containing the array of filters reflecting the current selections. This works for both the single and multiple select mode.Additionally the empty option (prompt option) is always filtered out, so when the prompt option is the only one selected, then the array will be empty, i.e. the select is re-set.
The demo under
web/examples/select.htmlwas very helpful :) I was wondering about was the behavior in multiple mode where the prompt option can be selected along with other options. Would the correct / expected behavior here be to override and deselect everything when that option is selected?In any case, the actual
onChangecallback's only responsibility is now to forward the selected options from the DOM element (and sort out browser differences). This also seems to make the_chart.onChangemethod more testable since one can easily call the method with different combinations of selections. I've left in the option to sendnullinto_chart.onChange, which is mostly a convenience in tests, but maybe confusing. Let me know if I should remove it.